Skip to content

Conversation

@arifbalik
Copy link
Member

@arifbalik arifbalik commented Nov 10, 2024

This is an implementation for STM32 TSC (touch sensing controller) driver. The details are discussed in

I've also added tests and a sample application for this and put it inside the same branch but this makes the PR rather large so I'm nor sure if I should split it in separate PRs, to make everything easier, I've separated them into different commits.

Closes #79126

Note: I've written this driver for my blog post. Maybe it can help the review process;

https://arifbalik.github.io/self-monorepo/zephyr-device-driver/

rruuaanng
rruuaanng previously approved these changes Nov 10, 2024
@kartben
Copy link
Contributor

kartben commented Nov 10, 2024

I may have missed something but why is it a misc driver and not an input one?

@arifbalik
Copy link
Member Author

I may have missed something but why is it a misc driver and not an input one?

I initially put it inside the input subsys and we've discussed about this in the issue I've linked. But it did not seem right because the acqusition needs to be started manually by calling stm32_tsc_start, and only after the acqusition we put the value to the input subsystem.

Also if the interrupts are kept disabled, the value needs to be polled, so it does not report it to the input subsys but rather user uses stm32_tsc_poll_for_acquisition and stm32_tsc_read to read the values.

Copy link
Member

@fabiobaltieri fabiobaltieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job though I think that implementing this as a misc driver and exposing it as counter is missing the point of using the input subsystem, or any subsystem really, you may as well use an entire custom API. An application would have to be written specifically for this device to use it anyway. The point of the input subsystem is that you can write an application that takes "buttons" and make it take events from any driver that generates a button event, and touch sense devices looks very much button-y to me.

I'd suggest dropping the sample, bringing the initialization code in the driver and make it generate key events.

@arifbalik
Copy link
Member Author

arifbalik commented Nov 10, 2024

Good job though I think that implementing this as a misc driver and exposing it as counter is missing the point of using the input subsystem, or any subsystem really, you may as well use an entire custom API. An application would have to be written specifically for this device to use it anyway. The point of the input subsystem is that you can write an application that takes "buttons" and make it take events from any driver that generates a button event, and touch sense devices looks very much button-y to me.

I'd suggest dropping the sample, bringing the initialization code in the driver and make it generate key events.

I can see how that gets confusing. Maybe I should drop input subsystem, because this peripheral is not exactly designed to only work as buttons but can also be used as a proximity sensor, sliders and whatnot, and usually a lot of filtrering is done over these count values, so acquiring them even every 1ms might be slow.

I designed this driver thinking that another library would run on this to finally produce a click or any other input event. Again maybe dropping it from this driver would be better, though I dont know where to put the captured data from the ISR

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Nov 10, 2024

I designed this driver thinking that another library would run on this to finally produce a click or any other input event. Again maybe dropping it from this driver would be better, though I dont know where to put the captured data from the ISR

You could keep this as a misc device with its own device specific API and then instead of the sample make an input driver based on top of the misc one that implements buttons (or sliders, or anything). Then one could use it for buttons with the input driver with no extra code, just dt configuration, or use it directly for whatever. You could even set a couple threshold and make it produce two input events, one for proximity and one for click, or more implementations, like one that implement a slider and produces absolute events. Would that make sense? I think it'd be cool, leave all options open but solves common use cases with in-tree code.

@arifbalik
Copy link
Member Author

arifbalik commented Nov 11, 2024

I designed this driver thinking that another library would run on this to finally produce a click or any other input event. Again maybe dropping it from this driver would be better, though I dont know where to put the captured data from the ISR

You could keep this as a misc device with its own device specific API and then instead of the sample make an input driver based on top of the misc one that implements buttons (or sliders, or anything). Then one could use it for buttons with the input driver with no extra code, just dt configuration, or use it directly for whatever. You could even set a couple threshold and make it produce two input events, one for proximity and one for click, or more implementations, like one that implement a slider and produces absolute events. Would that make sense? I think it'd be cool, leave all options open but solves common use cases with in-tree code.

Makes sense, so then I would do something like tsc-keys as in adc-keys, and it would take a tsc channel phandle, do some filtering and finally produce an input event

@zephyrbot zephyrbot added area: Devicetree area: Input Input Subsystem and Drivers labels Dec 1, 2024
@zephyrbot zephyrbot requested review from decsny and galak December 1, 2024 07:15
rruuaanng
rruuaanng previously approved these changes Feb 11, 2025
fabiobaltieri
fabiobaltieri previously approved these changes Feb 11, 2025
@fabiobaltieri
Copy link
Member

@gmarull @erwango can you take a look? would love to have this wrapped before RC1 on Friday

@fabiobaltieri
Copy link
Member

@gmarull can you take another look? Erwan is out if you approve this I think we can merge it before RC1

Added tsc pripheral bindings

Signed-off-by: Arif Balik <[email protected]>
update stm32u0 and stm32u083c_dk for TSC
peripheral

Signed-off-by: Arif Balik <[email protected]>
input_tsc_keys to detect key press releases
using STM32 TSC

Signed-off-by: Arif Balik <[email protected]>
integration tests for stm32 tsc

Signed-off-by: Arif Balik <[email protected]>
@JarmouniA JarmouniA requested a review from rruuaanng February 14, 2025 16:43
@kartben kartben modified the milestones: v4.1.0, v4.2.0 Feb 15, 2025
@kartben kartben merged commit 5d3113e into zephyrproject-rtos:main Mar 17, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

input: stm32 touch sensing controller peripheral implementation

9 participants